[gmp] Cleanup, fix cross-builds, switch windows from yasm to clang by dg0yt · Pull Request #27787 · microsoft/vcpkg

dg0yt · 2022-11-12T16:01:25Z

What does your PR fix?

Cleanup, fix cross-builds. Fixes [gmp] Build error on x64-linux-dynamic #27469.
Switch MSVC asm from yasm to clang, enabling asm for arm64-windows (based on commits and comments in [gmp] arm64 windows asm optimization #26764, [gmp] Use clang instead of yasm #23660, but only tested in CI).
Add feature fat for runtime selection of optimized low level routines on x86, cf. https://gmplib.org/manual/Notes-for-Package-Builds.
Which triplets are supported/not supported? Have you updated the CI baseline?

unchanged, no
Tested arm64-linux and mingw cross builds on x64-linux.
Does your PR follow the maintainer guide?

yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

yes

ports/gmp/tools.patch

ports/gmp/portfile.cmake

Neumann-A · 2022-11-12T22:57:13Z

ports/gmp/portfile.cmake

Is there a reason to not add this to PATH and keep the patch smaller ?

CURRENT_HOST_INSTALLED_DIR is only for cross builds. For native builds, the tools are in .. So

This port originally copied the host tools to the build dirs to avoid patching.

This PR uses the pattern I developed in [libgpg-error,libgcrpyt,libassuan,gpgme] Update, official mirrors, mingw, cross builds #27668: It transparently corrects the dependencies and executes the tool from its installed path. One benefit is that the path also clearly appears in the log.

I think the reason to add it to the path is again the spaces issue.

I see. But I wonder if space wouldn't cause problems in 200 other spots.
(I was already thinking about a PR which builds in /v pkg to see what breaks.)

I will check if I can use proper quotes.

The gmp install step fails anyways when the install root or packages root contains space, and there is a warning:

CMake Warning at scripts/cmake/vcpkg_configure_make.cmake:203 (message): Detected whitespace in root directory. Please move the path to one without whitespaces! The required tools do not handle whitespaces correctly and the build will most likely fail

The gmp build step fails anyways when the buildtrees contains space.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 6f7ffeb18f99796233b958aaaf14ec7bd4fb64b2 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 7428e7a..af37ffd 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2690,7 +2690,7 @@
     },
     "gmp": {
       "baseline": "6.2.1",
-      "port-version": 13
+      "port-version": 14
     },
     "gmsh": {
       "baseline": "4.9.0",
diff --git a/versions/g-/gmp.json b/versions/g-/gmp.json
index 8fd6673..b301b77 100644
--- a/versions/g-/gmp.json
+++ b/versions/g-/gmp.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "ff8dea9499f7141be457b57c4fef949a29aa941d",
+      "version": "6.2.1",
+      "port-version": 14
+    },
     {
       "git-tree": "28b5b46e27a69da50c1cd0a8be5d0a32cbca120b",
       "version": "6.2.1",

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/gmp/portfile.cmake

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 6f7ffeb18f99796233b958aaaf14ec7bd4fb64b2 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 7428e7a..af37ffd 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2690,7 +2690,7 @@
     },
     "gmp": {
       "baseline": "6.2.1",
-      "port-version": 13
+      "port-version": 14
     },
     "gmsh": {
       "baseline": "4.9.0",
diff --git a/versions/g-/gmp.json b/versions/g-/gmp.json
index 8fd6673..b45b8c2 100644
--- a/versions/g-/gmp.json
+++ b/versions/g-/gmp.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "e8d9f78bbb94c735ce2a7d1f644dd552e20aebd4",
+      "version": "6.2.1",
+      "port-version": 14
+    },
     {
       "git-tree": "28b5b46e27a69da50c1cd0a8be5d0a32cbca120b",
       "version": "6.2.1",

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/gmp/portfile.cmake

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 6f7ffeb18f99796233b958aaaf14ec7bd4fb64b2 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 7428e7a..af37ffd 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2690,7 +2690,7 @@
     },
     "gmp": {
       "baseline": "6.2.1",
-      "port-version": 13
+      "port-version": 14
     },
     "gmsh": {
       "baseline": "4.9.0",
diff --git a/versions/g-/gmp.json b/versions/g-/gmp.json
index 8fd6673..cb1e324 100644
--- a/versions/g-/gmp.json
+++ b/versions/g-/gmp.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "254ac1c99afc24f08c0932cb59d8a502785bc3fe",
+      "version": "6.2.1",
+      "port-version": 14
+    },
     {
       "git-tree": "28b5b46e27a69da50c1cd0a8be5d0a32cbca120b",
       "version": "6.2.1",

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/gmp/portfile.cmake

github-actions

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/gmp/portfile.cmake

Neumann-A · 2022-11-13T17:09:40Z

ports/gmp/msvc_symbol.patch

     if test "$enable_shared" = yes; then
-       GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined -Wl,--export-all-symbols"
+-      GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined -Wl,--export-all-symbols"
 -      LIBGMP_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmp-3.dll.def"
 -      LIBGMPXX_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmpxx-3.dll.def"
-+      #LIBGMP_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmp-3.dll.def"
-+      #LIBGMPXX_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmpxx-3.dll.def"
+      GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined"


hmm does the symbol export still work without these ?

I checked this with mingw-dynamic: The import libs are present and not empty, only smaller (in particular the C++ lib).
AFAIU from the headers, there should be enough export declarations for the public API. Only the tests, fuzzers etc. might need some symbols which are not covered. But vcpkg doesn't build them.
And the package even takes care of putting the right definitions into the header for DLL import. Maybe this is why they don't allow simultaneous shared+static for Windows.

dg0yt · 2022-11-13T13:27:01Z

ports/gmp/asmflags.patch

 # Flags used for preprocessing (in ansi2knr rules).
-diff --git a/tests/Makefile.in b/tests/Makefile.in


Note: Makefile.in is generated from Makefile.am.

dg0yt · 2022-11-13T17:55:29Z

ports/gmp/msvc_symbol.patch

     if test "$enable_shared" = yes; then
-       GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined -Wl,--export-all-symbols"
+-      GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined -Wl,--export-all-symbols"
 -      LIBGMP_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmp-3.dll.def"
 -      LIBGMPXX_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmpxx-3.dll.def"
-+      #LIBGMP_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmp-3.dll.def"
-+      #LIBGMPXX_LDFLAGS="$LIBGMP_LDFLAGS -Wl,--output-def,.libs/libgmpxx-3.dll.def"
+      GMP_LDFLAGS="$GMP_LDFLAGS -no-undefined"


I checked this with mingw-dynamic: The import libs are present and not empty, only smaller (in particular the C++ lib).
AFAIU from the headers, there should be enough export declarations for the public API. Only the tests, fuzzers etc. might need some symbols which are not covered. But vcpkg doesn't build them.
And the package even takes care of putting the right definitions into the header for DLL import. Maybe this is why they don't allow simultaneous shared+static for Windows.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/gmp/portfile.cmake

BillyONeal · 2022-11-18T20:58:02Z

Thanks for the gmp fixes! Happy to see yasm go bye bye bye

Neumann-A reviewed Nov 12, 2022

View reviewed changes

ports/gmp/tools.patch Outdated Show resolved Hide resolved

dg0yt force-pushed the gmp branch 2 times, most recently from f8842e7 to 1ee0428 Compare November 12, 2022 17:41

dg0yt added 3 commits November 12, 2022 19:34

[gmp] Fix non-msvc cross-builds

c01de3b

Fix msvc arm builds

8665a77

Stop exporting all symbols

d0620ae

dg0yt force-pushed the gmp branch from 1ee0428 to 45e1221 Compare November 12, 2022 22:24

Neumann-A reviewed Nov 12, 2022

View reviewed changes

ports/gmp/portfile.cmake Outdated Show resolved Hide resolved

Neumann-A reviewed Nov 12, 2022

View reviewed changes

Generalize ASMFLAGS patch

724b06b

dg0yt force-pushed the gmp branch from 45e1221 to 724b06b Compare November 13, 2022 09:06

github-actions bot requested changes Nov 13, 2022

View reviewed changes

dg0yt and others added 2 commits November 13, 2022 11:45

Replace yasm with clang, incl. arm64

f39ea32

Integrate arm64 patches from microsoft#26764

ddced27

github-actions bot requested changes Nov 13, 2022

View reviewed changes

Fix CCAS path setup

a7d1996

github-actions bot requested changes Nov 13, 2022

View reviewed changes

dg0yt added 4 commits November 13, 2022 12:33

Enable x86 runtime detection

d87c196

Skip readline detection

b37d2d9

Unroll tool names

634f652

Update versions

e711714

github-actions bot previously approved these changes Nov 13, 2022

View reviewed changes

dg0yt changed the title ~~[gmp] Cleanup, fix cross-builds~~ [gmp] Cleanup, fix cross-builds, switch windows from yasm to clang Nov 13, 2022

dg0yt marked this pull request as ready for review November 13, 2022 13:25

dg0yt mentioned this pull request Nov 13, 2022

[gmp] arm64 windows asm optimization #26764

Closed

Neumann-A reviewed Nov 13, 2022

View reviewed changes

dg0yt commented Nov 13, 2022

View reviewed changes

Modernize vcpkg_extract_source_archive_ex

8f90315

dg0yt dismissed github-actions[bot]’s stale review via 8f90315 November 13, 2022 18:14

github-actions bot approved these changes Nov 13, 2022

View reviewed changes

FrankXie05 assigned Adela0814 Nov 14, 2022

Adela0814 assigned Cheney-W Nov 14, 2022

Adela0814 added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-bug The issue is with a library, which is something the port should already support labels Nov 14, 2022

Cheney-W approved these changes Nov 18, 2022

View reviewed changes

Cheney-W added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Nov 18, 2022

BillyONeal merged commit e461f6d into microsoft:master Nov 18, 2022

dg0yt deleted the gmp branch November 18, 2022 21:12

LilyWangLL mentioned this pull request Nov 23, 2022

[gmp] Build error - 'fat' does not build on Windows x64 #27957

Closed

		# Flags used for preprocessing (in ansi2knr rules).
		diff --git a/tests/Makefile.in b/tests/Makefile.in

Conversation

dg0yt commented Nov 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Uh oh!

Uh oh!

Uh oh!

Neumann-A Nov 12, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

Neumann-A Nov 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Neumann-A Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Nov 13, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

BillyONeal commented Nov 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dg0yt commented Nov 12, 2022 •

edited

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

Neumann-A Nov 13, 2022 •

edited

Loading

dg0yt Nov 13, 2022 •

edited

Loading